Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: workflow authoring and management support #487

Merged
merged 121 commits into from
Feb 9, 2024

Conversation

mikeee
Copy link
Member

@mikeee mikeee commented Dec 17, 2023

Implements Workflow authoring using durabletask-go and also an interface to the dapr grpc management endpoints.

Contributes towards the completion of dapr/dapr#7156 and closes #379

This PR also exposes the GRPC connection from the dapr client.

Requires documentation in dapr/docs#3895

  • Testing implemented (E2E)
  • Checks/Tests passing
  • Documentation updated/improved

@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch from facf3ee to 3ae03a4 Compare December 17, 2023 15:48
Copy link

codecov bot commented Dec 17, 2023

Codecov Report

Attention: 290 lines in your changes are missing coverage. Please review.

Comparison is base (cefbadb) 70.11% compared to head (6f5aa89) 59.37%.
Report is 1 commits behind head on main.

Files Patch % Lines
examples/workflow/main.go 0.00% 182 Missing ⚠️
workflow/client.go 50.61% 38 Missing and 2 partials ⚠️
workflow/worker.go 55.38% 25 Missing and 4 partials ⚠️
workflow/workflow.go 54.16% 21 Missing and 1 partial ⚠️
workflow/context.go 50.00% 17 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #487       +/-   ##
===========================================
- Coverage   70.11%   59.37%   -10.74%     
===========================================
  Files          35       54       +19     
  Lines        2884     3483      +599     
===========================================
+ Hits         2022     2068       +46     
- Misses        748     1293      +545     
- Partials      114      122        +8     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@mikeee mikeee changed the title feat: workflow authoring support feat: workflow authoring support [very wip] Dec 19, 2023
@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch from d9f8f1c to 74cbdbc Compare January 2, 2024 17:24
@mikeee mikeee mentioned this pull request Jan 4, 2024
3 tasks
@mikeee mikeee changed the title feat: workflow authoring support [very wip] feat: workflow authoring and management support Jan 4, 2024
@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch 18 times, most recently from f166caf to 328c94a Compare January 9, 2024 15:45
@mikeee mikeee marked this pull request as ready for review January 11, 2024 00:25
@mikeee mikeee requested a review from a team as a code owner January 11, 2024 00:25
Copy link

@mukundansundar mukundansundar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great start with adding in workflow support with Dapr management APIs. Just left a few comments and suggestions.

  • consider adding authoring SDK
  • should generics support be added instead of any interface{} for input/output as if there are plans to add it later on that might be a breaking change? If breaking changes are ok, then this can be a future feature.

cc @yaron2 wdyt?

client/client.go Outdated Show resolved Hide resolved
examples/workflow/README.md Outdated Show resolved Hide resolved
@mikeee mikeee force-pushed the mikeee-feat-workflow-authoring branch 3 times, most recently from f5fa833 to 947414d Compare January 14, 2024 14:39
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee requested a review from cgillum January 30, 2024 12:36
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@yaron2
Copy link
Member

yaron2 commented Jan 31, 2024

Looks like we can merge this. @cgillum

Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working my way through this. Adding a few comments now before I finish reviewing.

client/actor.go Outdated Show resolved Hide resolved
client/workflow.go Outdated Show resolved Hide resolved
client/workflow.go Show resolved Hide resolved
Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple super-minor things. The most important is probably changing the wording in the documentation comments regarding .Await(...) as I'm a bit worried that users might misunderstand how tasks can be used.

client/workflow.go Outdated Show resolved Hide resolved
workflow/context.go Outdated Show resolved Hide resolved
mikeee and others added 3 commits February 7, 2024 10:53
Co-authored-by: Chris Gillum <cgillum@gmail.com>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@mikeee mikeee requested a review from cgillum February 7, 2024 12:47
Copy link
Contributor

@cgillum cgillum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm signing off on this but with a couple final recommendations.

I appreciate the hard work that went into this. :)

client/workflow.go Show resolved Hide resolved
workflow/context.go Outdated Show resolved Hide resolved
Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
- task invoke documentation
- refactor type assertion for startworkflowbeta1

Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@mikeee
Copy link
Member Author

mikeee commented Feb 7, 2024

@yaron2 for your eyes

@yaron2
Copy link
Member

yaron2 commented Feb 8, 2024

@mikeee please resolve the conflicts and we can merge

@mikeee
Copy link
Member Author

mikeee commented Feb 8, 2024

I had a feeling this would happen 😂

Signed-off-by: mikeee <hey@mike.ee>
Signed-off-by: mikeee <hey@mike.ee>
@mikeee
Copy link
Member Author

mikeee commented Feb 8, 2024

@yaron2 good to go, not sure why codecov is unhappy but 🤷

@yaron2 yaron2 merged commit ac26e62 into dapr:main Feb 9, 2024
13 of 15 checks passed
@mikeee mikeee deleted the mikeee-feat-workflow-authoring branch February 9, 2024 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workflow implementation in SDK
5 participants